-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[llvm-exegesis] Error instead of aborting on verification failure #137581
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[llvm-exegesis] Error instead of aborting on verification failure #137581
Conversation
This patch makes llvm-exegesis emit an error when the machine function fails in MachineVerification rather than aborting. This allows downstream users (particularly https://github.com/google/gematria) to handle these errors rather than having the entire process crash. This essentially be NFC from the user perspective minus the addition of the new error message.
|
@llvm/pr-subscribers-tools-llvm-exegesis Author: Aiden Grossman (boomanaiden154) ChangesThis patch makes llvm-exegesis emit an error when the machine function fails in MachineVerification rather than aborting. This allows downstream users (particularly https://github.com/google/gematria) to handle these errors rather than having the entire process crash. This essentially be NFC from the user perspective minus the addition of the new error message. Full diff: https://github.com/llvm/llvm-project/pull/137581.diff 1 Files Affected:
diff --git a/llvm/tools/llvm-exegesis/lib/Assembler.cpp b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
index f638478e0c51d..67f3d75a5e879 100644
--- a/llvm/tools/llvm-exegesis/lib/Assembler.cpp
+++ b/llvm/tools/llvm-exegesis/lib/Assembler.cpp
@@ -29,6 +29,7 @@
#include "llvm/MC/MCInstrInfo.h"
#include "llvm/Object/SymbolSize.h"
#include "llvm/Support/Alignment.h"
+#include "llvm/Support/Error.h"
#include "llvm/Support/MemoryBuffer.h"
#include "llvm/Support/raw_ostream.h"
@@ -323,10 +324,8 @@ Error assembleToStream(const ExegesisTarget &ET,
TPC->printAndVerify("After ExegesisTarget::addTargetSpecificPasses");
// Adding the following passes:
// - postrapseudos: expands pseudo return instructions used on some targets.
- // - machineverifier: checks that the MachineFunction is well formed.
// - prologepilog: saves and restore callee saved registers.
- for (const char *PassName :
- {"postrapseudos", "machineverifier", "prologepilog"})
+ for (const char *PassName : {"postrapseudos", "prologepilog"})
if (addPass(PM, PassName, *TPC))
return make_error<Failure>("Unable to add a mandatory pass");
TPC->setInitialized();
@@ -337,6 +336,10 @@ Error assembleToStream(const ExegesisTarget &ET,
return make_error<Failure>("Cannot add AsmPrinter passes");
PM.run(*Module); // Run all the passes
+ bool MFWellFormed =
+ MF.verify(nullptr, "llvm-exegesis Assembly", &outs(), false);
+ if (!MFWellFormed)
+ return make_error<Failure>("The machine function failed verification.");
return Error::success();
}
|
|
@mshockwave Bump on this when you get a chance. Thanks! |
mshockwave
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry this slipped through my radar. LGTM.
This includes llvm/llvm-project#137581 which makes it a bit nicer to deal with verification failures, some of which come from inline assembly and would be difficult to fix within LLVM.
This includes llvm/llvm-project#137581 which makes it a bit nicer to deal with verification failures, some of which come from inline assembly and would be difficult to fix within LLVM.
This patch makes llvm-exegesis emit an error when the machine function fails in MachineVerification rather than aborting. This allows downstream users (particularly https://github.com/google/gematria) to handle these errors rather than having the entire process crash. This essentially be NFC from the user perspective minus the addition of the new error message.